Skip to content

Replace appender in stata #62621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Replace appender in stata #62621

wants to merge 5 commits into from
+148 −81

Conversation

JuanCarlos3
Copy link

@JuanCarlos3 JuanCarlos3 commented Oct 7, 2025 •
edited
Loading

Removed @appender on the following functions:

pandas.io.stata.StataReader.read
pandas.io.stata.read_stata

@JuanCarlos3 JuanCarlos3 closed this Oct 7, 2025
@JuanCarlos3 JuanCarlos3 reopened this Oct 7, 2025
@JuanCarlos3 JuanCarlos3 closed this Oct 8, 2025
@JuanCarlos3 JuanCarlos3 reopened this Oct 8, 2025
mroeschke
mroeschke reviewed Oct 8, 2025
View reviewed changes
pandas/io/stata.py
size = self._chunksize
return self.read(nrows=size)

@Appender(_read_method_doc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can _read_method_doc (and any associated variables) be removed after this change?

Copy link
Author

@JuanCarlos3 JuanCarlos3 Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Matthew, yes the _read_method_doc can be removed. Should I remove it in this PR?

pandas/io/stata.py


@set_module("pandas")
@Appender(_read_stata_doc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can _read_stata_doc (and any associated variables) be removed after this change?

Copy link
Author

@JuanCarlos3 JuanCarlos3 Oct 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Matthew, yes the _read_stata_doc and _iterator_params can be removed. Should I remove it in this PR?

@mroeschke mroeschke added the Docs label Oct 8, 2025
@JuanCarlos3
Copy link
Author

JuanCarlos3 commented Oct 9, 2025 •
edited
Loading

Hi Matthew, thanks for reviewing my PR! You’re right—some of those strings can be removed. I initially kept them because I hadn’t seen other PRs deleting them, and the original issue comments mentioned a broader cleanup effort after all decorators were removed. But I’m happy to remove them as part of this PR if that’s preferred.

I had a couple of questions I was hoping you could answer:

  1. Since this is my first contribution, I kept the PR intentionally small. Please let me know if it’s too minor and whether you’d prefer that I also remove the @doc decorator from stata.py.

  2. I noticed a few CI/CD checks are failing. From what I can tell in the logs, it might be related to reopening the PR multiple times, which could have caused an artifact ID conflict in the pipeline. Could you please confirm if there’s anything in my code that needs to be updated, or advise on the standard approach for resolving this type of issue?

@mroeschke
Copy link
Member

But I’m happy to remove them as part of this PR if that’s preferred

Yup go ahead and perform those cleanups in this PR. You can address the @doc cleanups in a separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke left review comments

Assignees
No one assigned
Labels
Docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DOC: Replace @Appender, @Substitution, @doc with inlined docstrings
2 participants
@JuanCarlos3 @mroeschke